Add terminate selected tasks action in admin by caoqianming · Pull Request #449 · celery/django-celery-results

caoqianming · 2024-10-18T02:26:02Z

No description provided.

auvipy

can you please elaborate the change a bit more? can you also verify the change?

caoqianming · 2024-10-21T09:52:50Z

When a long task is in running state, I may need to terminate it for various reasons. So adding this action in admin is useful. I added the code and tested it and it works fine. In the beginning I was terminating tasks one by one in a for loop, later I realized that I could pass in a list, so I made some changes.

auvipy

can you add tests to verify this? and do not introduce any regression?

caoqianming · 2024-10-21T10:02:21Z

Okay, I'll try.

caoqianming · 2024-10-21T10:54:23Z

can you add tests to verify this? and do not introduce any regression?您能否添加测试来验证这一点？并且不引入任何回归？

Sorry, I have no idea to write this UNIT test. And i run python manage.py test with some errors in the picture. can you help me or give some suggestion, thank you

AllexVeldman · 2025-02-28T16:37:10Z

@caoqianming unittests are run using pytest.

install a virtualenv with the test dependencies, make sure you are on python 3.11:

python -m venv .venv
. .venv/bin/activate
pip install -r requirements/default.txt -r requirements/test.txt -r requirements/test-django.txt
pytest

This requires a postgres database to be available on localhost with USER postgres PASSWORD postgres and a database with the name postgres. I normally use a docker image I already have running, or just run the plain docker image: https://hub.docker.com/_/postgres

Currently there are no tests testing the admin pages since there was very little functionality there, so you'll have to add a test_admin.py yourself.
If you run into issues creating the tests, ping me here and I'll see if I can spend some time on this.

for more information, see https://pre-commit.ci

caoqianming · 2025-04-12T15:05:20Z

Hello. I add some test @AllexVeldman

Copilot

Pull Request Overview

This PR adds a new action to the Django admin interface allowing administrators to terminate selected tasks, along with corresponding unit tests.

Introduces terminate_task action in TaskResultAdmin which terminates tasks and reports success or error messages.
Adds unit tests in t/unit/test_admin.py covering both successful termination and failure scenarios.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
t/unit/test_admin.py	Adds tests for the new terminate_task action
django_celery_results/admin.py	Implements the terminate_task action in the admin

Comments suppressed due to low confidence (1)

t/unit/test_admin.py:27

[nitpick] The variable name 'id' shadows the built-in function id(). Consider renaming it to 'task_id' or similar to avoid potential confusion.

id = uuid()

auvipy · 2025-06-17T13:02:33Z

I am going to deeply consider it again, may be by the end of this month.

auvipy

can you please fix the merge conflict on test file?

for more information, see https://pre-commit.ci

…tion definition and remove trailing blank line at end of file

auvipy

looks good

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

django_celery_results/admin.py

Copilot · 2026-03-07T13:13:42Z

django_celery_results/admin.py

+            self.message_user(
+                request,
+                f"{len(task_ids)} Task was terminated successfully.",
+                messages.SUCCESS,
+            )


The success message is grammatically incorrect for counts != 1 (e.g., "2 Task was...") and is not translatable. Consider using Django’s pluralization utilities (e.g., ngettext) and wrapping the message in translations so admin UI strings are localized consistently with the rest of this module.

use task(s)

Copilot · 2026-03-07T13:13:42Z

django_celery_results/admin.py

+        except Exception as e:
+            self.message_user(
+                request,
+                f"Error while terminating tasks: {e}",
+                messages.ERROR,
+            )


Catching Exception and interpolating the raw exception into a user-facing admin message can leak internal details. Prefer logging the exception (with stack trace) and showing a generic failure message, or at least sanitizing/normalizing the error output for admins.

please check this @caoqianming if this is right or not

add err log, but still keep err message

Copilot · 2026-03-07T13:13:43Z

django_celery_results/admin.py

+                messages.ERROR,
+            )
+
+    terminate_task.short_description = "Terminate selected tasks"


terminate_task.short_description is a user-facing admin string but it isn’t wrapped in gettext_lazy (the module already uses _() for other UI text). Consider making this translatable to keep admin UI localization consistent.

Copilot · 2026-03-07T13:13:43Z

t/unit/test_admin.py

+    @patch('celery.current_app.control.terminate')
+    def test_terminate_task_success(self, mock_terminate):


The patch target should generally point at where the symbol is used. Since terminate_task() calls django_celery_results.admin.celery_app.control.terminate, patching celery.current_app.control.terminate may not reliably intercept the call (depending on how Celery’s current_app proxy is resolved). Patch the admin module reference instead to make the test robust.

@caoqianming and this as well

Copilot · 2026-03-07T13:13:43Z

t/unit/test_admin.py

+        self.assertEqual(
+            str(messages[0]),
+            "2 Task was terminated successfully.")
+        self.assertEqual(messages[0].level, constants.SUCCESS)


The expected success message asserts the same singular/plural grammar issue as the implementation ("2 Task was..."). Once the admin message is fixed to pluralize correctly, this assertion should be updated accordingly (and ideally made resilient using pluralization utilities).

t/unit/test_admin.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…esults

for more information, see https://pre-commit.ci

Nusnus requested a review from auvipy October 19, 2024 21:21

Add terminate selected tasks in action

266e62c

caoqianming force-pushed the main branch from b05057f to 266e62c Compare October 21, 2024 08:53

auvipy requested changes Oct 21, 2024

View reviewed changes

caoqianming requested a review from auvipy October 21, 2024 09:54

auvipy requested changes Oct 21, 2024

View reviewed changes

caoqianming and others added 4 commits April 12, 2025 22:32

Add test for terminate selected tasks

0a183d4

[pre-commit.ci] auto fixes from pre-commit.com hooks

7a19716

for more information, see https://pre-commit.ci

fix flake8 check

8555e3b

fix flake8 check2

f645f4c

auvipy requested a review from Copilot May 12, 2025 07:22

Copilot AI reviewed May 12, 2025

View reviewed changes

auvipy approved these changes May 12, 2025

View reviewed changes

auvipy self-requested a review June 17, 2025 13:02

auvipy requested changes Oct 2, 2025

View reviewed changes

auvipy and others added 4 commits October 2, 2025 18:24

Merge branch 'main' into main

cd2f238

[pre-commit.ci] auto fixes from pre-commit.com hooks

c595d0c

for more information, see https://pre-commit.ci

Merge branch 'celery:main' into main

3cb2340

Fix flake8 errors in test_admin.py: add missing blank line after func…

b2c6b75

…tion definition and remove trailing blank line at end of file

auvipy requested a review from Copilot March 7, 2026 13:09

Copilot started reviewing on behalf of auvipy March 7, 2026 13:10 View session

auvipy reviewed Mar 7, 2026

View reviewed changes

Copilot AI reviewed Mar 7, 2026

View reviewed changes

Update django_celery_results/admin.py

271ff60

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

auvipy and others added 6 commits March 7, 2026 19:15

Update t/unit/test_admin.py

0c6f04d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update t/unit/test_admin.py

b157f7c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix test failures in test_admin.py and adjust admin message

fe24849

Merge branch 'main' of https://github.com/caoqianming/django-celery-r…

3752473

…esults

Fix E402 import order and update patch target in tests

47f8698

[pre-commit.ci] auto fixes from pre-commit.com hooks

eb949bc

for more information, see https://pre-commit.ci

		@patch('celery.current_app.control.terminate')
		def test_terminate_task_success(self, mock_terminate):

Conversation

caoqianming commented Oct 18, 2024

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

caoqianming commented Oct 21, 2024

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

caoqianming commented Oct 21, 2024

Uh oh!

caoqianming commented Oct 21, 2024

Uh oh!

AllexVeldman commented Feb 28, 2025

Uh oh!

caoqianming commented Apr 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

auvipy commented Jun 17, 2025

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

caoqianming Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

auvipy Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

caoqianming Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

auvipy Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

caoqianming Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

auvipy Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants